Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes 2129. #2168

Closed
Closed

Conversation

ibolmo
Copy link
Member

@ibolmo ibolmo commented Dec 12, 2011

IE < 9, has issues with caching and getAttribute('width'). If the image
is cached, the width will be specified as soon as you assign the
.src attribute using the cached dimensions. This only happens for
dynamically creates images. IE handles pre-defined elements as expected.

Added Element.src.html spec with instructions on how to test.

Fixed old manual specs that references old Event.js. Updated
Specs/index.html to include older manual specs and the new
Element.src.html.

PASSED: IE6-9

@ibolmo
Copy link
Member Author

ibolmo commented Dec 12, 2011

Fixes #2129.

@@ -579,6 +579,19 @@ propertyGetters['class'] = function(node){
return ('className' in node) ? node.className || null : node.getAttribute('class');
};

/* <ltIE9> */
if (Browser.ie && Browser.version < 9) propertySetters.src = function(node, value){
var isNew = !node.parentNode,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdalton this was the only line that I wasn't happy with. Do you know of a better way to check if an Element is "new".

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ga really it's 2011, enough with the UA sniffs.

In IE a removed element will still have a parentNode so you can check it against the parentElement property.
(parentElement will be null once detached from the document unlike parentNode)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hehe remember that there's no cheap way to check for image cache bug that we're trying to normalize. I guess I could use Google Analytics blank.gif but what's the point if I know that the behavior is ltIE9.

Thanks for the tip. Do you think that should suffice the isNew? I guess I could temporarily flag it in new Element and toggle it in src.

Oh and I forgot to check against img. I'll do this in a bit.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hehe remember that there's no cheap way to check for image cache bug that we're trying to normalize. I guess I could use Google Analytics blank.gif but what's the point if I know that the behavior is ltIE9.

Except the whole thing is a doc note, not a "cache bug", and not really a patch concern for MooTools.
The problem is that unlike other browsers IE will assign height and width attributes automatically for a loaded image. So when you change the src it will enforce the attributes it set. MooTools has no way to determine if the user or IE has set those attributes.

IE < 9, has issues with caching and getAttribute('width'). If the image
is cached, the width will be `specified` as soon as you assign the
`.src` attribute using the cached dimensions. This only happens for
dynamically creates images. IE handles pre-defined elements as expected.

Added `Element.src.html` spec with instructions on how to test.

Fixed old manual specs that references old `Event.js`. Updated
`Specs/index.html` to include older manual specs and the new
`Element.src.html`.

PASSED: IE6-9
Pull-push: what if the element was in DOM and then disposed and reset
src to another. This is buggy since IE resets the width/height even if
the width or height were specified. This fix normalizes.

PASSED: IE6-9
@ibolmo
Copy link
Member Author

ibolmo commented Feb 6, 2012

Opting for doc.

@ibolmo ibolmo closed this Feb 6, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants